Skip to content

fix(auth): confidential clients may sign in without PKCE, as OpenID certification needs - #521

Open
PiquelChips wants to merge 1 commit into
mainfrom
fix/pkce-confidential
Open

PiquelChips wants to merge 1 commit into
mainfrom
fix/pkce-confidential

Conversation

@PiquelChips

@PiquelChips PiquelChips commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Auth required S256 PKCE from every client. A confidential client that sends none, as the OpenID conformance suite's Basic and logout plans do, failed at its first authorization, so 37 of their tests could check nothing behind it.

Now a confidential client may leave PKCE out, as RFC 9700 §2.1.1 allows: it authenticates with its secret when it redeems the code. Nothing gets weaker for the rest:

  • Public clients must still send an S256 challenge.
  • A challenge a confidential client sends is enforced like any other.
  • A code issued without a challenge can't be redeemed with a code_verifier, so an attacker can't strip PKCE from a request (downgrade).
  • The SDKs always send PKCE.

pkce_challenge becomes nullable on pending requests and codes (migration 0048). The OIDC reference, the app guides and the security docs say who needs PKCE.

Closes #514

Note

Written by Claude Opus 5.5 in Claude Code (via T3 Code)

@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6227eb6f-60a5-42fa-9f88-3b9a19d542cb
Commits

Reviewing files that changed from the base of the PR and between 13d749d and b7aecc2.

Files selected for processing: 21

  • apps/auth-service/src/deletion/tests.rs
  • apps/auth-service/src/oidc/authorize/parameters.rs
  • apps/auth-service/src/oidc/tests/authorization.rs
  • apps/auth-service/src/oidc/token.rs
  • apps/auth-service/src/tests.rs
  • crates/database/.sqlx/query-42ac9c52f8c14b06434dccc3564591b2c73a8adbaf5bc25db5fe0afa34995bf6.json
  • crates/database/.sqlx/query-a35642846dde2778d46c8f41779fceb2f261b43d1566b88f638ef89fdf736ad5.json
  • crates/database/.sqlx/query-b55c00b8d90ca13fe21b71bd171bf41a8ac9169e67ddb23346b5c9d285849012.json
  • crates/database/.sqlx/query-c2caf3e062bcf46312ba760ccaf9f58f30a3de8c66102f5d3fd92f3339872cc9.json
  • crates/database/.sqlx/query-f2738a32dac0d1b49340ecbc82b2d0a4fc9ddf1b34b5b1003ed84f08d73156ac.json
  • crates/database/migrations/0048_optional_pkce.sql
  • crates/database/src/repositories/oidc/mod.rs
  • crates/database/src/repositories/oidc/tests.rs
  • docs/dev/architecture/oidc.md
  • docs/dev/operations/clients.md
  • docs/dev/security/boundaries.md
  • docs/dev/security/threat-model.md
  • docs/user/apps/how-sign-in-works.md
  • docs/user/apps/register-an-app.md
  • docs/user/apps/troubleshooting.md
  • docs/user/reference/oidc.md
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Every authorization request had to carry an S256 challenge, so a
confidential client that sends none, as OpenID certification's Basic and
logout plans do, failed at its first authorization. Following RFC 9700
§2.1.1, a confidential client may now leave PKCE out: it authenticates with
its secret at the token endpoint. Public clients must still send a
challenge, a challenge sent is always enforced, and a code issued without
one can't be redeemed with a code_verifier, so a request can't be
downgraded.

Closes #514
@PiquelChips
PiquelChips force-pushed the fix/pkce-confidential branch from c492db2 to b7aecc2 Compare November 13, 2026 22:30
@PiquelChips
PiquelChips removed this pull request from stack #525 November 13, 2026 22:30
@PiquelChips
PiquelChips changed the base branch from ci/conformance-suite to main November 13, 2026 22:31
@PiquelChips
PiquelChips added this pull request to stack #530 November 13, 2026 22:31
@PiquelChips PiquelChips self-assigned this Nov 13, 2026
@PiquelChips PiquelChips added bug Something isn't working apps/auth-service Origin Auth at auth.sourceorigin.dev, its settings and developer console, and the operator CLI labels Nov 13, 2026
@PiquelChips PiquelChips added this to the v0.1 milestone Nov 13, 2026
@github-project-automation github-project-automation Bot moved this to Needs Triage in origin-src.dev Nov 13, 2026
@PiquelChips PiquelChips moved this from Needs Triage to In Progress in origin-src.dev Nov 13, 2026
@PiquelChips PiquelChips added standards Open standards compliance (OIDC, OAuth, RFCs) schema-change Any changes to the database schema labels Nov 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

apps/auth-service Origin Auth at auth.sourceorigin.dev, its settings and developer console, and the operator CLI bug Something isn't working schema-change Any changes to the database schema standards Open standards compliance (OIDC, OAuth, RFCs)

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

fix(auth): PKCE is optional for confidential clients

1 participant